Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent feature corruption in LRU cache #4650

Merged
merged 10 commits into from
Jan 23, 2024

Conversation

mrucker
Copy link
Contributor

@mrucker mrucker commented Oct 9, 2023

The LRU Cache in the EMT reduction maintains references to memories within the EMT tree. These references could become corrupted when the ownership of the memory pointer was transferred to the tree leaf. This update fixes this problem by waiting until after ownership is transferred before storing the memory reference in the cache.

@ataymano
Copy link
Member

That seems like workaround but not fix.
I tried to add (it is clearly not copyable since it is has vector of unique_ptrs):

emt_node(const emt_node&) = delete;
emt_node& operator=(const emt_node&) = delete;

and it broke my build (cmake + vs2019 + win)

@ataymano ataymano merged commit b4612f8 into VowpalWabbit:master Jan 23, 2024
116 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants